-
-
Notifications
You must be signed in to change notification settings - Fork 154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Auto-detect Github Actions CI and activate github logger accordingly #1645
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Build failure
it's not your fault, but comes from Anyway, I'm happy to merge, please update the docs. |
See feedback in infection#1645 (comment)
Thanks! I've pushed three more changes:
|
looks like our end-to-end tests are broken now (https://github.com/infection/infection/runs/4787795564?check_suite_focus=true#step:12:5) I suspect this is because tests, executed from PHPUnit, started producing GitHub warning messages (https://github.com/infection/infection/runs/4787795564?check_suite_focus=true#step:12:27) and this wasn't exptected from PHPUnit context I have a feeling that we need to disable github logger here. Since it's being enabled automatically, probably we need to make it possible to pass infection/tests/phpunit/Console/E2ETest.php Line 351 in 07490ad
so the logic would be:
what do you think? Let me know if you need any help, since e2e tests can be not very clear. |
b25559f
to
788a432
Compare
@maks-rafalko what a gotcha 😅 I've pushed a commit to support
I hope this is what you had in mind; I had to change it to nullable etc. Though I've to trouble to figure out where to explicitly put |
private function detectCiGithubActions(): bool | ||
{ | ||
try { | ||
$ci = $this->ciDetector->detect(); | ||
|
||
if ($ci->getCiName() === CiDetector::CI_GITHUB_ACTIONS) { | ||
return true; | ||
} | ||
} catch (CiNotDetectedException $e) { | ||
// deliberately empty | ||
} | ||
|
||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private function detectCiGithubActions(): bool | |
{ | |
try { | |
$ci = $this->ciDetector->detect(); | |
if ($ci->getCiName() === CiDetector::CI_GITHUB_ACTIONS) { | |
return true; | |
} | |
} catch (CiNotDetectedException $e) { | |
// deliberately empty | |
} | |
return false; | |
} | |
private function detectCiGithubActions(): bool | |
{ | |
try { | |
$ci = $this->ciDetector->detect(); | |
} catch (CiNotDetectedException $e) { | |
return false; | |
} | |
if ($ci->getCiName() !== CiDetector::CI_GITHUB_ACTIONS) { | |
return false; | |
} | |
return true; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️
src/Command/RunCommand.php
Outdated
throw new InvalidArgumentException( | ||
sprintf('Cannot pass "%s" to "--%s": only "true", "false" or no argument is supported', $useGitHubLogger, self::OPTION_LOGGER_GITHUB) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw new InvalidArgumentException( | |
sprintf('Cannot pass "%s" to "--%s": only "true", "false" or no argument is supported', $useGitHubLogger, self::OPTION_LOGGER_GITHUB) | |
); | |
throw new InvalidArgumentException(sprintf( | |
'Cannot pass "%s" to "--%s": only "true", "false" or no argument is supported', | |
$useGitHubLogger, | |
self::OPTION_LOGGER_GITHUB | |
)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️
@mfn could you please rebase? |
See feedback in infection#1645 (comment)
788a432
to
fe91e83
Compare
Done and force-pushed! |
See feedback in infection#1645 (comment)
fe91e83
to
1dcd2f1
Compare
I've rebased and fixed e2e tests. Thanks for this feature @mfn! |
Thank you for your time too 🙏🏼 |
This PR:
Fixes #1560
With this PR, the
--logger-github
flag is not necessary when running in Github Actions anymore. It's still supported:When running on Github Actions, however, it's not possible to "disable" this logger; as long as the Github Actions CI is correctly detected according to OndraM/ci-detector, it will always be active. This is deliberately done to improve the out of the box experience with infection.